Add Client Metadata Update Support. #1708

vbabanin · 2025-05-06T07:06:54Z

Description

This PR updates the Java sync and reactive-streams driver to support dynamic client metadata updates, enabling wrapping libraries and integrations to append metadata about themselves when they do not instantiate the MongoClient directly.

JAVA-5870

NOTE BEFORE MERGING:

Remove a submodule link to a fork: #1708 (comment)

JAVA-5854

vbabanin · 2025-05-08T05:25:19Z

buildSrc/src/main/kotlin/conventions/testing-base.gradle.kts

+dependencies {
+    testImplementation(libs.assertj)
+}


AssertJ has been used in this PR and added to test-base as it is a useful library that could be shared across all modules.

A decision to use a new assertion library seems like something that should be debated a bit more widely, rather than being brought in as part of a PR like this one. How important do you think it is to the assertions required for this PR?

I agree that adopting a new assertion or testing approach should generally be a broader team decision. This isn’t a critical for this PR, so I can remove it from this PR. AssertJ has already been used in the Kotlin coroutine module (reference) and is present in the toml file, though it’s not widely used in the codebase. This PR made the library visible to all modules, based on its usefulness in the Hibernate repository, which was the motivation for extending its usage further.

For now, I will revert this change and we can revisit the discussion as a team.

JAVA-5854

JAVA-5870

jyemin

Initial feedback

jyemin · 2025-05-20T20:58:11Z

driver-sync/src/main/com/mongodb/client/MongoClient.java

+     * @param mongoDriverInformation the driver information to append to the existing metadata
+     * @since 5.6
+     */
+    void updateMetadata(MongoDriverInformation mongoDriverInformation);


What's the argument for putting the method on MongoClient rather than MongoCluster?

Just conceptually, MongoCluster acts as a view with overridden configurations (e.g., codec registry, read preference) and doesn’t own the client’s lifecycle or global state. In my opinion, placing updateMetadata on MongoCluster raises the question: does it update metadata for just that instance or for all instances? That behavior would require clear documentation.

MongoClient already handles driver-wide operations (e.g., close), so updateMetadata aligns with its role.

jyemin · 2025-05-20T21:00:29Z

driver-core/src/main/com/mongodb/internal/connection/ClientMetadata.java

+ * <p>This class is not part of the public API and may be removed or changed at any time</p>
+ */
+public class ClientMetadata {
+    private volatile BsonDocument clientMetadataBsonDocument;


I don't thing that a volatile fields is enough. If append is called concurrently by multiple threads, then one of the updates to metadata will likely be lost.

You're right - volatile alone isn't enough for concurrent updates. I initially didn’t add locking because I expected concurrent metadata updates to be extremely rare in practice, but I’ve now added a lock to ensure consistency. Let me know what you think.

UPD: discussed offline. Removed clone() from updateMetadata(...) for clarify: 8ade58b

JAVA-5870

nhachicha · 2025-05-27T11:14:21Z

driver-core/src/main/com/mongodb/internal/connection/ClientMetadata.java


 /**
 * Represents metadata of the current MongoClient.
 *
 * <p>This class is not part of the public API and may be removed or changed at any time</p>
 */
 public class ClientMetadata {
-    private final ReentrantLock updateLock = new ReentrantLock();
-    private volatile BsonDocument clientMetadataBsonDocument;


If you remove the volatile, then I think you need to use the updateLock.readLock() in getBsonDocument

Yes, I intended to use the readLock() here. Thanks for catching that - updated accordingly!

nhachicha · 2025-06-04T12:48:26Z

driver-core/src/main/com/mongodb/internal/connection/ClientMetadata.java

@@ -36,7 +36,7 @@ public class ClientMetadata {
    private BsonDocument clientMetadataBsonDocument;

    public ClientMetadata(@Nullable final String applicationName, final MongoDriverInformation mongoDriverInformation) {
-        withLock(readWriteLock.writeLock(), () -> {
+        withLock(readWriteLock.readLock(), () -> {


why switching to a readLock()?

Ah, that was an oversight on my part - I meant to apply the change in getBsonDocument . Fixed here:
Fix readLock issue.

vbabanin · 2025-06-04T16:59:47Z

I switched from volatile (for reads) and volatile + lock (for writes) to a ReadWriteLock. While the original approach was sufficient for thread safety, ReadWriteLock should be easier to understand at a glance without sacrificing performance, as the overhead is negligible.

vbabanin · 2025-06-10T05:11:44Z

.gitmodules

+	url = https://github.com/vbabanin/specifications
+	branch = DRIVERS-2985


A link to the fork was added to connect the specification PR under review with this PR, letting unified tests to run. This should be removed once the specification PR is merged and before merging this PR.

Reverted back: 31ef18e

Synced with https://github.com/mongodb/specifications upstream master: 38ba5e6

JAVA-5870

# Conflicts: # driver-core/src/test/resources/specifications # driver-core/src/test/unit/com/mongodb/internal/connection/MultiServerClusterSpecification.groovy

# Conflicts: # driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java

…fication.

nhachicha

Not sure why the changes of SDAM/Connection/Monitoring is part of this PR? The client metadata aspect LGTM but didn't review the SDAM changes

nhachicha · 2025-06-18T09:32:13Z

driver-core/src/main/com/mongodb/internal/connection/AbstractMultiServerCluster.java

@@ -269,7 +270,7 @@ private boolean handleReplicaSetMemberChanged(final ServerDescription newDescrip
        }

        if (isStalePrimary(newDescription)) {
-            invalidatePotentialPrimary(newDescription);
+            invalidatePotentialPrimary(newDescription, new MongoStalePrimaryException("Primary marked stale due to electionId/setVersion mismatch"));


Is there a test asserting this exception?

nhachicha · 2025-06-18T13:41:09Z

...-core/src/test/unit/com/mongodb/internal/connection/DefaultServerMonitorSpecification.groovy

why it show up empty in the diff?

jyemin · 2025-06-18T14:35:25Z

buildSrc/src/main/kotlin/conventions/testing-base.gradle.kts

+dependencies {
+    testImplementation(libs.assertj)
+}


A decision to use a new assertion library seems like something that should be debated a bit more widely, rather than being brought in as part of a PR like this one. How important do you think it is to the assertions required for this PR?

jyemin · 2025-06-18T14:40:42Z

driver-core/src/main/com/mongodb/internal/connection/ClientMetadata.java

+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static com.mongodb.internal.Locks.withLock;
+import static com.mongodb.internal.connection.ClientMetadataHelper.createClientMetadataDocument;


Now that we have a proper ClientMetadata class that is meant to be instantiated, it seems like we no longer need a separate ClientMetadataHelper class, instead moving the methods of that class into this one.

jyemin · 2025-06-18T14:56:44Z

driver-core/src/main/com/mongodb/internal/connection/ClientMetadataHelper.java

+     * <p>
+     * Platform is appended separately to the existing platform if it does not exceed {@value MAXIMUM_CLIENT_METADATA_ENCODED_SIZE} bytes.
+     */
+    public static BsonDocument updateClientMetadataDocument(final BsonDocument clientMetadataDocument,


Let's consider a slightly different algorithm for updating client metadata that shared more logic with createClientMetadataDocument.

Now that we have a ClientMetadata class that is meant to be instantiated, we can store more than just the resulting BsonDocument representing the metadata. We can also store the data from which it was created: the application name and the MongoDriverInformation. If we have that, we can share all the create logic by first merging the original MongoDriverInformation with the appended one, and then calling createClientMetadataDocument. We might need a new MongoDriverInformation#merge method, or it could be added to the MongoDriverInformation.Builder, but otherwise it reduces the code complexity a bit.

What do you think?

jyemin · 2025-06-18T15:02:26Z

driver-core/src/main/com/mongodb/internal/connection/ClientMetadata.java

+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
+import static com.mongodb.internal.Locks.withLock;
+import static com.mongodb.internal.connection.ClientMetadataHelper.createClientMetadataDocument;


It's also surprising that ClientMetadataHelper.createClientMetadataDocument is called directly in multiple places, e.g. com.mongodb.MongoClient, instead of just getting the ClientMetadata from the cluster or delegate. Looks like it's done just to log the metadata. Could we simplify that?

vbabanin · 2025-06-19T07:04:58Z

Not sure why the changes of SDAM/Connection/Monitoring is part of this PR? The client metadata aspect LGTM but didn't review the SDAM changes

After merging main into this branch, some diffs (SDAM-related changes) are appearing, even though they’re already present on main and the total number of line additions hasn’t changed. I’m looking into why this is happening.

vbabanin added 3 commits May 6, 2025 00:03

Add Client Metadata update support.

9753f28

Add prose tests.

889d5c8

JAVA-5854

Add prose tests.

4b3065c

JAVA-5854

vbabanin self-assigned this May 8, 2025

vbabanin commented May 8, 2025

View reviewed changes

Add ClientMetadata entity.

2a684bf

JAVA-5854

vbabanin requested review from a team and nhachicha and removed request for a team May 8, 2025 05:55

vbabanin added 7 commits May 7, 2025 23:54

Update tests.

28c1844

JAVA-5854

Merge branch 'main' into JAVA-5854

371ce0b

Fix tests.

972fe9d

JAVA-5854

Fix tests.

bcf9cc8

JAVA-5854

Spotless fix.

179b262

JAVA-5854

Skip tests when auth is enabled.

bc43ba0

JAVA-5870

Apply Scala spotless.

3f5fc91

JAVA-5870

vbabanin mentioned this pull request May 9, 2025

DRIVERS-2985: Allow on-demand client metadata updates after MongoClient initialization. mongodb/specifications#1798

Merged

3 tasks

vbabanin added 7 commits May 9, 2025 17:12

Update tests.

61192d8

JAVA-5870

Fix tests.

e9f8dd6

JAVA-5870

Add parametrized tests.

95c1bb1

JAVA-5870

Apply Kotlin spotless.

dd63a49

JAVA-5870

Move update to separate variables.

89d67bb

JAVA-5870

Merge branch 'main' into JAVA-5854

8a18d39

Merge branch 'main' into JAVA-5854

f296d0c

vbabanin requested a review from jyemin May 20, 2025 20:58

jyemin reviewed May 20, 2025

View reviewed changes

vbabanin added 2 commits May 20, 2025 20:04

Add lock for updates.

a8dc4fb

JAVA-5870

Clone document before passing it as an argument.

8ade58b

JAVA-5870

vbabanin requested a review from jyemin May 21, 2025 19:19

Rename to "appendMetadata".

71350fa

JAVA-5870

Add ReadWriteLock and rename method.

9890aa1

JAVA-5870

vbabanin requested a review from nhachicha May 27, 2025 07:18

nhachicha reviewed May 27, 2025

View reviewed changes

vbabanin added 2 commits June 3, 2025 23:56

Add parametrized tests.

28f7d88

Add readLock.

246a040

vbabanin requested a review from nhachicha June 4, 2025 07:01

nhachicha reviewed Jun 4, 2025

View reviewed changes

Fix readLock issue.

8a58294

vbabanin requested a review from nhachicha June 4, 2025 17:00

vbabanin added 4 commits June 4, 2025 10:02

Merge branch 'main' into JAVA-5854

15ecef8

Add Kotlin and Scala API.

5c7a6e3

Add unified tests.

f466d08

Merge branch 'main' into JAVA-5854

3132541

vbabanin commented Jun 10, 2025

View reviewed changes

vbabanin added 6 commits June 9, 2025 22:17

Fix static checks.

d961447

Change specification submodule commit.

74d8558

Revert specifications submodule URL to upstream repository.

31ef18e

JAVA-5870

Update specifications submodule commit hash.

38ba5e6

Merge branch 'main' into JAVA-5854

edafc54

# Conflicts: # driver-core/src/test/resources/specifications # driver-core/src/test/unit/com/mongodb/internal/connection/MultiServerClusterSpecification.groovy

Merge branch 'main' into JAVA-5854

6aa0a1e

# Conflicts: # driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java

vbabanin marked this pull request as ready for review June 17, 2025 03:02

vbabanin requested a review from a team as a code owner June 17, 2025 03:02

Remove duplicate import of CLIENT_METADATA in MultiServerClusterSpeci…

dab5451

…fication.

nhachicha reviewed Jun 18, 2025

View reviewed changes

jyemin requested changes Jun 18, 2025

View reviewed changes

Merge branch 'main' into JAVA-5854

1550122

vbabanin added 2 commits June 19, 2025 21:06

Remove ClientMetadataHelper.

b5c4c20

Update native-image.properties.

b6b4d67

		url = https://github.com/vbabanin/specifications
		branch = DRIVERS-2985

Add Client Metadata Update Support. #1708

Are you sure you want to change the base?

Add Client Metadata Update Support. #1708

Conversation

vbabanin commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

NOTE BEFORE MERGING:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbabanin Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jyemin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbabanin May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbabanin May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nhachicha May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbabanin Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbabanin commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbabanin Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nhachicha left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbabanin commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vbabanin commented May 6, 2025 •

edited

Loading

vbabanin Jun 20, 2025 •

edited

Loading

vbabanin May 21, 2025 •

edited

Loading

vbabanin May 21, 2025 •

edited

Loading

nhachicha May 27, 2025 •

edited

Loading

vbabanin Jun 4, 2025 •

edited

Loading

vbabanin commented Jun 4, 2025 •

edited

Loading

vbabanin Jun 17, 2025 •

edited

Loading

vbabanin commented Jun 19, 2025 •

edited

Loading